Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix glitch in SpringBoneSimulator3D by storing the previous frame's rotation instead of using no rotation when the axis is flipped #101651

Merged
merged 1 commit into from
Jan 17, 2025

Conversation

TokageItLab
Copy link
Member

@TokageItLab TokageItLab commented Jan 16, 2025

The SkeletonModifier discards the bone pose after skin rendered, so no rotation in the next frame means the pose is not affected by that SkeletonModifier at all. In order to keep the previous pose, the SkeletonModifier needs to store the previous frame's state by itself.

When axis is flipped, currently SpringBoneSimulator returns no rotation Quaternion(0, 0, 0, 1), which makes it glitchy because it means resetting the pose.

fix01.mp4

This PR allows the SpringBoneSimulator to store the previous frame's rotation. Then, if the rotation axis may be inverted, it doesn't move and prevents glitches. For further stability, a check for inversion by dot product has been added.

Without checking dot product (The right most shakes for a moment):

fix02.mp4

With checking dot product (Current PR):

fix03.mp4

@TokageItLab TokageItLab added this to the 4.4 milestone Jan 16, 2025
@TokageItLab TokageItLab requested a review from a team as a code owner January 16, 2025 21:42
@TokageItLab TokageItLab changed the title Changed SpringBoneSimulator to return the previous frame's rotation instead of no rotation when the axis is flipped. Changed SpringBoneSimulator3D to store the previous frame's rotation instead of no rotation when the axis is flipped Jan 16, 2025
Copy link
Member

@fire fire left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is necessary for 4.4 because it reduces a visual animation glitch.

@TokageItLab TokageItLab changed the title Changed SpringBoneSimulator3D to store the previous frame's rotation instead of no rotation when the axis is flipped Changed SpringBoneSimulator3D to store the previous frame's rotation instead of using no rotation when the axis is flipped Jan 16, 2025
@fire fire requested a review from a team January 16, 2025 21:44
@fire
Copy link
Member

fire commented Jan 16, 2025

This warning, as an error, breaks the GitHub action tests.


./scene/3d/spring_bone_simulator_3d.cpp:1582:34: error: ISO C++ says that these are ambiguous, even though the worst conversion for the first is better than the worst conversion for the second: [-Werror]
 1582 |         if (Math::is_equal_approx(p_from.dot(p_to), -1.0f)) {
      |             ~~~~~~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~
In file included from ./core/math/vector3.h:35,
                 from ./core/math/plane.h:34,
                 from ./core/math/aabb.h:34,
                 from ./core/variant/variant.h:37,
                 from ./core/object/message_queue.h:38,
                 from ./core/object/object.h:35,
                 from ./core/variant/typed_array.h:34,
                 from ./scene/main/node.h:35,
                 from ./scene/3d/node_3d.h:34,
                 from ./scene/3d/audio_listener_3d.h:34,
                 from ./scene/3d/audio_listener_3d.cpp:31,
                 from scene/3d/scu/scu_scene_3d.gen.cpp:1:
./core/math/math_funcs.h:597:37: note: candidate 1: 'static bool Math::is_equal_approx(double, double)'
  597 |         static _ALWAYS_INLINE_ bool is_equal_approx(double a, double b) {
      |                                     ^~~~~~~~~~~~~~~
./core/math/math_funcs.h:571:37: note: candidate 2: 'static bool Math::is_equal_approx(float, float)'
  571 |         static _ALWAYS_INLINE_ bool is_equal_approx(float a, float b) {
      |                                     ^~~~~~~~~~~~~~~

@TokageItLab TokageItLab changed the title Changed SpringBoneSimulator3D to store the previous frame's rotation instead of using no rotation when the axis is flipped Fix glitch in SpringBoneSimulator3D to store the previous frame's rotation instead of using no rotation when the axis is flipped Jan 16, 2025
@TokageItLab TokageItLab changed the title Fix glitch in SpringBoneSimulator3D to store the previous frame's rotation instead of using no rotation when the axis is flipped Fix glitch in SpringBoneSimulator3D by storing the previous frame's rotation instead of using no rotation when the axis is flipped Jan 16, 2025
@Repiteo Repiteo merged commit 4425ce0 into godotengine:master Jan 17, 2025
20 checks passed
@Repiteo
Copy link
Contributor

Repiteo commented Jan 17, 2025

Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

3 participants